-
Notifications
You must be signed in to change notification settings - Fork 2.1k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
receive: allow custom connection pooling #1966
receive: allow custom connection pooling #1966
Conversation
d313c74
to
b61fb0a
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
👍 awesome stuff
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
👍 awesome stuff
Signed-off-by: Brett Jones <blockloop@users.noreply.github.com>
f5851b3
to
4904950
Compare
I think the build needs to be kicked @bwplotka |
Somewhat related, I was thinking we should switch the internal communications to use gRPC, which could eventually evolve to also be the externally accepting write request API. I’m mentioning this as that would probably make this obsolete. What do you think? |
I agree. I think this should be gRPC streams all the way through. I have a branch on my fork with a lot of that work complete. https://github.com/blockloop/thanos/tree/bjones/receive/use-grpc |
Happy to review a PR switching the internal forwarding to gRPC. I say
switching rather than adding as there is no use for two internal forwarding
mechanisms.
El El mié, ene. 8, 2020 a las 19:07, Brett <notifications@github.com>
escribió:
… I agree. I think this should be gRPC streams all the way through. I have a
branch on my fork with a lot of that work complete.
https://github.com/blockloop/thanos/tree/bjones/grpc-remote-write
—
You are receiving this because you commented.
Reply to this email directly, view it on GitHub
<#1966?email_source=notifications&email_token=AE4JAPYZ54SBK2MTLEAZ253Q4YI6JA5CNFSM4KELD3O2YY3PNVWWK3TUL52HS4DFVREXG43VMVBW63LNMVXHJKTDN5WW2ZLOORPWSZGOEINOORA#issuecomment-572188484>,
or unsubscribe
<https://github.com/notifications/unsubscribe-auth/AE4JAP62GE2RXXGVR5PEF4LQ4YI6JANCNFSM4KELD3OQ>
.
|
@squat, I think that makes sense. Right now since forwarding and receiving are the same code that would require a bit of a change. I can change gears a bit and start on splitting away forwarding from receiving HTTP remote writes. That might be the better option. Ideally the gRPC endpoint would also accept remote writes externally as well. |
Let me know if you want to pair. I think it should be quite straight
forward to essentially keep the same forward func and simply have two
handlers (one gRPC and one HTTP) that both call the same forwarding func.
El El mié, ene. 8, 2020 a las 21:30, Brett <notifications@github.com>
escribió:
… @squat <https://github.com/squat>, I think that makes sense. Right now
since forwarding and receiving are the same code that would require a bit
of a change. I can change gears a bit and start on splitting away
forwarding from receiving HTTP remote writes. That might be the better
option. Ideally the gRPC endpoint would also accept remote writes
externally as well.
—
You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub
<#1966?email_source=notifications&email_token=AE4JAP7UI5TOYHBCWQ7ONSDQ4YZXNA5CNFSM4KELD3O2YY3PNVWWK3TUL52HS4DFVREXG43VMVBW63LNMVXHJKTDN5WW2ZLOORPWSZGOEIN342A#issuecomment-572243560>,
or unsubscribe
<https://github.com/notifications/unsubscribe-auth/AE4JAPYCDZRBE6HE3DAHDQ3Q4YZXNANCNFSM4KELD3OQ>
.
|
Yeah I think a RemoteWrite method needs to be added to |
Maybe we should shift the conversation to #1710 and close this PR? |
Yeah it’ll have to be a new service that has the store methods as well as the RemoteWrite method |
Closing this in favor of #1970 |
Changes
Receive: enable custom connection pool sizing. With the default configuration machines with very high load (over 500 requests per second) will cycle TCP connections too frequently and run out of available ports. See more here
Verification
Running a custom build in production. Also see #1955